Add headings and other geometric pins in motion using State_tags#3995
Add headings and other geometric pins in motion using State_tags#3995rodw-au wants to merge 61 commits into
Conversation
merge latest upstream into state-tags-arcs branch
merged to upstream
merge master to state-tag-arcs branch
…d read active_modes() in rs274ngc_pre.cc still ran the original unclamped loop above the clamped one, so the stack overflow into saved_settings[5] was unchanged. The line-number copy was also commented out. Drop the duplicate and restore the line-number copy. write_state_tag() still dereferenced block on the iscircle assignment (`block->iscircle = (block == NULL) ? 0 : block->iscircle`), reintroducing the M70 NULL-deref. Replace with a plain null-guarded read. control.c output_to_hal() read the iscircle bit out of `tag.fields[]` using the StateFlag index GM_FLAG_IS_CIRCLE, which indexes past `fields[]` (GM_FIELD_MAX_FIELDS == 8). Read it from packed_flags instead, like the update_status() copy does. Also dropped the duplicate interp_normal_heading write in the G2/G3 branch and restored the WATCH_FLAGS debug block to live inside update_status().
Round 2: re-fix M70 segfault and iscircle handling
M_PI_2 is a glibc extension and is not exposed by RTAI's rtapi_math.h, so the RTAI build fails with 'M_PI_2 undeclared' at control.c:2283. M_PI is available in both, so use M_PI/2.0 instead.
Fix RTAI build: M_PI_2 undeclared
|
Some observations:
|
|
Thanks for the feedback.
Ideas to resolve Item 3 would be appreciated. Just reviewing further, it appears #3924 that the ~/linuxcnc/configs folder contents have been added to the root folder on my PC. I don't think they have been included in this PR. Could someone please check and confirm? |
…that was not resolved when work commenced on this PR. 2. Deleted spurious arc_heading file. 3. Moved docs/man/man9/output_buffer.9 to docs/man/.gitignore
Sorry my fault, I forgot to add the generated man page to gitignore (#3941) |
|
Hm, I'm not sure I've been looking too good at the nc directory... Point Point 3 should be a separate PR, I guess. Point 4 needs to be solved by you taking the original file and only change what is necessary. |
|
Just to add a comment on nc files. It is not that you cannot or should not add (example) files. The point is that there needs to be some kind of docs or procedures that use or reference them. Just having files without people knowing how to find them is kinda pointless. |
You are not the first who gets bitten by this. There is a change in the works to let CI fail when this happens. |
As mentioned in the original comment, these new pins are documented in the motion man file eg Re point 2, you now see the problem with the nc_files folder caused by #3924 . I too only saw it it when the files were removed by git add. It was very wrong as it created the files that should have been under ~/linuxcnc in whatever was the current working directory. So saving the sim to your desktop created this mess! |
The man page is a reference for the component. These man pages usually do not do a lot of examples. For that we have the other documentation in the docs/src directory, where specific scenarios are described and more in depth explanations are (should be) presented. This is usually also the place, while explaining and going into depth, to reference the examples provided.
Yes, there was that unfortunate wrong side-effect while fixing the Tcl9 compatibility problem, sorry about that. However, it should be fixed now, isn't it? |
Man pages should find their way, yes. So does the other documentation afaik. |
in preparation to fix format errors caused by editor auto format options Aditional commit to follow
then reapplying chnagfes in this PR to it
|
Formatting issue in I can't see a clean way to correct the .gitignore issue by a PR from the oversight in #3941. Its been corrected in this PR. Re documentation, when I look at a the docs page on the web site, I can't see any chapter where additional documentation of a few extra pins is appropriate except for the man page. Perhaps these gcode samples could be deployed and mentioned on the man page but where do I put them so they turn up in the users nc_file examples? Adding example gcode for read only pins just does not seem right to me. The man page should be sufficient (as it is for every other component). All runtests pass I think this is now OK for approval. It is benign as it does not add any additional features, just exposes some useful information. Breakages are unlikely. Any fixes following user feedback can be added. |
You were correct about the divergence. It happened in the first batch of commits and it must have finally bitten. Now fixed. I noticed you corrected the tolerance. But I think its now looser on an inch machine thjan a metric machine. Not sure how to fix that. Can you pass a paramter to the ui file? I guess it won't really matter becasue if its off it will be a G20/G21 issue and it will be far more than that! |
I noticed you had fixed that. |
|
Oops Fixed command.c. Was looking in the wrong file (control.c) and found some redundant code anyway |
|
Finally! All cleaned up. Sorry about the stream of commits. I'm very relieved our revised gcode passes all tests Thanks for all your help and suggestions. It will be great to see this committed now. Some users are keen for it as it was a forum post that spurred me on to get this done. I had an attempt about 6 years ago and gave up but it was good to start afresh with this commit as things had changed and I had done even more silly things than I did in this time! |
|
Looks fine to me ;-) |
|
@andypugh you need to squash the commits. |
6 participants
Outline
create new [motion.interp.xx ] pins for heading and various geometric data (refer man motion for this PR). Note the following use cases behind this PR.
I am sure the community will find many other use cases.
Code notes
Caveats
We use the state tag motion_type (GM_FIELD_MOTION_MODE) to detect G0,G1,G2,G3 in motion. Currently there is no way to use existing interpreter equates for these values so the numbers are hard coded as 0,10,20,30.
RunTests
These pass locally
Acknowledgements
Many thanks to Luca Toniolo (grandixximo) for his encouragement and help resolving errors with the tests.